Repository navigation
Conversation
|
Some changes occurred in compiler/rustc_codegen_llvm/src/debuginfo cc @Walnut356 Some changes occurred in compiler/rustc_codegen_llvm/src/builder/autodiff.rs cc @ZuseZ4 Some changes occurred in coverage instrumentation. cc @Zalathar |
rustbot
added
A-LLVM
|
rustbot has assigned @dingxiangfei2009. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
|
This is a large number of individual changes mixed together (despite the modest line count), some of which are more contentious than others, which makes reviewing this a bit of a slog. I'd suggest separating the different kinds of changes into separate commits, to make it easier to check that each kind of change looks good. |
| let value_words = [value as u64, (value >> 64) as u64]; | ||
| let size_in_bits = size.bits(); | ||
| // LLVM computes `NumWords = (SizeInBits + 63) / 64`. | ||
| assert!((size_in_bits + 63) / 64 <= value_words.len() as u64); |
There was a problem hiding this comment.
This assertion is deliberately written to match the LLVM computation, and should not be changed, since the change makes it harder to verify that both sides are performing the same computation.
| feature_err( | ||
| self.sess(), | ||
| feature, | ||
| span, | ||
| "C-variadic function definitions on this target are unstable", | ||
| ) | ||
| .emit(); |
There was a problem hiding this comment.
This seems worse than the status quo, as the call is now spread across several lines without an increase in clarity. I would at least retain msg as a local variable to make the call more compact.
| // Set up the small-data optimization limit for architectures that use | ||
| // an LLVM argument to control this. | ||
| if let (Some(threshold), SmallDataThresholdSupport::LlvmArg(arg)) = ( |
There was a problem hiding this comment.
If we're bothering to clean up this match, then IMO it should become an if-let chain rather than an odd if-let on a tuple.
| pub(crate) fn set_struct_body(&self, ty: &'ll Type, els: &[&'ll Type]) { | ||
| unsafe { llvm::LLVMStructSetBody(ty, els.as_ptr(), els.len() as c_uint, FALSE) } | ||
| } | ||
|
|
||
| pub(crate) fn set_packed_struct_body(&self, ty: &'ll Type, els: &[&'ll Type]) { | ||
| unsafe { llvm::LLVMStructSetBody(ty, els.as_ptr(), els.len() as c_uint, TRUE) } | ||
| } | ||
|
|
There was a problem hiding this comment.
This is the sort of change that should be presented in its own commit at least, not bundled in with dozens of trivial changes.
23fa2fa to
c4fab04
Compare
|
These commits modify compiler targets. Some changes occurred in GUI tests. This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp. Some changes occurred in HTML/CSS/JS. cc @GuillaumeGomez, @lolbinarycat Some changes occurred in src/tools/compiletest cc @jieyouxu Some changes occurred in src/tools/cargo cc @weihanglo |
rustbot
added
A-compiletest
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
oh no, i messed up a rebase. let me fix it |
c4fab04 to
04cc725
Compare
This comment has been minimized.
This comment has been minimized.
|
Sorry to everyone who got pinged 😅 I should've split these up into commit from the start. |
This comment has been minimized.
This comment has been minimized.
04cc725 to
6fa07e2
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
6fa07e2 to
3ba6ed9
Compare
|
☔ The latest upstream changes (presumably #165844) made this pull request unmergeable. Please resolve the merge conflicts by rebasing. |
small clippy cleanups